Fixed CSS transparency bug in _style_properties.py#5890
Conversation
|
Aha, so its is making the alpha 1 regardless? That looks like it would fix the case of transparent, but I think what we want to do is multiply the alpha. This would permit a color variable with an existing alpha, plus an additional alpha. Something like "rgba(100,20,20,0.5) 50%" How about replacing the |
|
Yeah that's exactly right, It was just setting it to the default alpha of 1.0 for all named colors which includes "transparent". Nice solution with |
|
Awesome. Can I request a snapshot test? |
|
Yes that would be appropriate! Sorry didn't occur to me that this is related to visual output so a snapshot would be in order. That might take me a bit, I'm not super familiar with the snapshot system. |
|
Let me know if you need any pointers. |
|
I have added a snapshot test. Marked it as a regression test linking to this PR, I believe that's what I was supposed to do. Love the snapshot system btw, I'll be using that more often now. |
|
Thanks |
Please review the following checklist.
I believe I've found the fix for this issue as originally reported in the Discord help channel.
So first off, original MRE showcasing the problem since, as requested, we're going straight to PR without an attached issue:
The issue was that in the
ColorPropertiesclass in css/_style_properties.py, it was not accounting for the case where a named color might have alpha built into it. Which is of course only one named color, "transparency".Line in question:
textual/src/textual/css/_style_properties.py
Line 994 in 6e4f77b
In the fix, I've added an extra line:
Previously it would only get the alpha
if token.endswith("%"), which was missing the edge case where the string is "transparent". Now after parsing the string, it extracts whatever alpha value is to re-use it for theparsed_color.with_alpha(alpha)call.I thought this was a nice solution since its a one-line addition, which is a bit more elegant than making an extra condition just for the "transparent" string. But regardless, I'm pretty sure this is the root of the issue. Let me know if you think this is maybe not the best way to fix it.